Skip to content

feat(bootstrap, react/homepage): #COCO-5441, school widget#468

Open
jcbe-ode wants to merge 13 commits intoepic-homepagefrom
feat/COCO-5441-school-widget
Open

feat(bootstrap, react/homepage): #COCO-5441, school widget#468
jcbe-ode wants to merge 13 commits intoepic-homepagefrom
feat/COCO-5441-school-widget

Conversation

@jcbe-ode
Copy link
Copy Markdown
Contributor

@jcbe-ode jcbe-ode commented Mar 23, 2026

Description

  • Adds a new SchoolSpace component for the React homepage module (with styling, Storybook stories, and tests).

  • Adds a hook to help populating the widget properties.

  • Adds a container encapsulating both the hook and component.

  • Adds a useWidgetPreferences for the homepage to load users preferences about widgets.

  • Updates the client School session type to reflect nullable exports (as seen during local tests).

Ticket https://edifice-community.atlassian.net/browse/COCO-5441

Which Package changed?

Please check the name of the package you changed

  • Components
  • Core
  • Icons
  • Hooks

Has the documentation changed?

  • Storybook

Type of change

Please check options that are relevant.

  • Chore (PATCH)
  • Doc (PATCH)
  • Bug fix (PATCH)
  • New feature (MINOR)
  • Breaking change (MAJOR)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new “SchoolWidget” for the React homepage module (with styling, Storybook stories, and a helper hook) and updates the client School session type to reflect nullable exports.

Changes:

  • Introduce SchoolWidget component + Storybook stories for single/multiple/empty states.
  • Add useUserSchools hook to derive schools/selected school from EdificeClientProvider.
  • Update School.exports typing in the client session interfaces (string[] | null) and add widget CSS/SVG background.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/react/src/modules/homepage/components/SchoolWidget/useUserSchools.ts New hook to read user schools and manage selected school state.
packages/react/src/modules/homepage/components/SchoolWidget/SchoolWidget.tsx New UI widget rendering selected school + dropdown when multiple schools.
packages/react/src/modules/homepage/components/SchoolWidget/SchoolWidget.stories.tsx Storybook coverage for multiple/single/empty scenarios.
packages/react/src/modules/homepage/components/SchoolWidget/SchoolWidget.css Widget styling + CSS variables + background image binding.
packages/react/src/modules/homepage/components/SchoolWidget/SchoolWidgetBackground.svg Background asset for the widget.
packages/client/src/ts/session/interfaces.ts Update School.exports to allow null from the API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@damienromito damienromito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Une décision à prendre sur l'endroit où mettre le style. Discussion à avoir avec @pascalsaussier-edifice

En terme de design j'attend confirmation de Simon https://www.figma.com/design/UbAKNC5vk5XOj62z6CI4Qm?node-id=2146-36161#1690237762
Mais ça sera probablement :

  • si le texte est plus grand que le container, ajouter un elipsis au lieu de passer sur deux le lignes
  • le background doit être sticky en haut du block et pas en bas


const hasManySchools = schools && schools.length > 1;

const widgetStyle = { padding: '1.4rem 0.4rem' };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mettre le css en dehors du composant et au même endroit, dans le fichier .scss (cf commentaire precedent). C'est un point important à acter pour le rester du projet

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tout ce style doit être dans un fichier CSS ou SCSS.
Les seuls styles inlines qu'on va s'autoriser sont ceux qui sont calculés en fonction d'un state ou d'une variable du composant.

const widgetStyle = { padding: '1.4rem 0.4rem' };
const containerStyle = { padding: '0.8rem' };
const selectedSchoolStyle = {
padding: '.4rem 2.9rem',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pour les espacement, dans les application on utiliser les classes bootstrap (p-8, p12,...), dans le FF il faut utiliser les spacer (qui ne peut que être utilisé dans les fichiers sass dans bootstrap justement, exemple)

Ici il s'agit d'un spacer 1.2 (12pixel sur figma)

Image

color="tertiary"
variant="ghost"
icon={
<IconRafterDown
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wahou chiant d'avoir dû reimplementer ça. J'ai fait un commentaire à simon https://www.figma.com/design/UbAKNC5vk5XOj62z6CI4Qm?node-id=2146-36161#1690319223

Comment on lines +95 to +104
export const Empty: Story = {
render: renderWithProps({ selectedSchool: undefined }),
parameters: {
docs: {
description: {
story: `État vide (aucune école)`,
},
},
},
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est vraiment utile ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'aime bien avoir la possibilité de voir les Edge Case dans storybook, ça retire pas mal de questionnements quand on regarde le composant

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est ce que le cas avec aucune ecole est possible ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non, mais en cas d'erreur de paramétrage (provenant de la BDD) autant que le widget ne crashe pas la page !

@@ -0,0 +1,122 @@
// packages/react/src/modules/homepage/components/SchoolWidget/useUserSchools.test.tsx
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comme pour le style, je pense qu'a la compilation , l'image ne sera pas ajoutée aux assets (tester en utilisant le composant dans Actualites par exemple).
Je vois qu'il y a un dossier react/modules/icons , sinon ajouter dans le dossier /bootstrap.
Quelle est la bonne pratique @pascalsaussier-edifice ?

setSelectedSchool(
index < 0 || index >= schools.length ? schools[0] : schools[index],
);
} else {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est ce vraiment utile de gerer ce cas ? c'est possible fonctionnellement qu'il passe à undefined ?

Copy link
Copy Markdown
Contributor Author

@jcbe-ode jcbe-ode Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • L'utilisateur choisit une structure => le choix est enregistré en BDD
  • Une année passe, ou bien il changé d'établissement en cours d'année
    => à la reconnexion, le choix précédemment enregistré n'est plus disponible dans la liste
    ==> on affiche le 1er étab de la liste à la place

border-radius: 2.4rem;

background-color: var(--school-widget-bg-color);
background-image: url('./SchoolWidgetBackground.svg');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est une image qui va être lié aux préférénces de couleur qui seront accessibles dans le panneau de configuration. Pas en v1 mais faudra y penser.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. bien vu, mais c'est un comportement qu'on va devoir mettre en place à plusieurs endroit. Tu as une idée de comment faire ? on peut passer des proriétés à un svg ?

Comment on lines +95 to +104
export const Empty: Story = {
render: renderWithProps({ selectedSchool: undefined }),
parameters: {
docs: {
description: {
story: `État vide (aucune école)`,
},
},
},
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'aime bien avoir la possibilité de voir les Edge Case dans storybook, ça retire pas mal de questionnements quand on regarde le composant


const hasManySchools = schools && schools.length > 1;

const widgetStyle = { padding: '1.4rem 0.4rem' };
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tout ce style doit être dans un fichier CSS ou SCSS.
Les seuls styles inlines qu'on va s'autoriser sont ceux qui sont calculés en fonction d'un state ou d'une variable du composant.

cdnDomain?: string | null;
version?: string | null;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci pour le nettoyage !

globals: true,
environment: 'jsdom',
include: ['src/**/*.spec.tsx'],
include: ['src/**/*.spec.tsx', 'src/**/*.spec.ts'],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question pour plus tard, sur les sites on utilise plutôt .test.ts[x] le .spec.ts[x] c'est surtout un héritage d'Angular, est-ce que ça vaudrait pas le coup de normaliser avec le .test ?
@damienromito

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi pas, test est plus explicite en effet, mais il faudra le changer partout ^^

@jcbe-ode jcbe-ode changed the title feat(react/homepage): #COCO-5441, school widget feat(bootstrap, react/homepage): #COCO-5441, school widget Mar 30, 2026
@jcbe-ode jcbe-ode force-pushed the feat/COCO-5441-school-widget branch 2 times, most recently from 07e420b to 62004ee Compare March 31, 2026 12:57
@jcbe-ode jcbe-ode force-pushed the feat/COCO-5441-school-widget branch from 62004ee to 5874ab8 Compare March 31, 2026 13:05
@jcbe-ode jcbe-ode force-pushed the feat/COCO-5441-school-widget branch from 9248f7f to dcae84e Compare March 31, 2026 13:54
@use '../../vendors/bootstrap';

.school-widget {
--border-color: #ffe5a3;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comme les variables sont scopées, on peut raccourcir leur nom sans risquer la collision avec les variables des autres composants.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh pas bête. Comme pour des variables en JS je ne trouve pas forcement utile quand on a une seule instance d'une variable, mais ça n'affecte pas la lisibilité donc ça me va

@jcbe-ode
Copy link
Copy Markdown
Contributor Author

Tous les retours sont traités autant que possible.
Qu'en pensez-vous ?

@jcbe-ode jcbe-ode requested a review from damienromito March 31, 2026 14:05
Copy link
Copy Markdown
Member

@damienromito damienromito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rien de bloquant ! top, un bon exemple de composant

@use '../../vendors/bootstrap';

.school-widget {
--border-color: #ffe5a3;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh pas bête. Comme pour des variables en JS je ne trouve pas forcement utile quand on a une seule instance d'une variable, mais ça n'affecte pas la lisibilité donc ça me va

@use '../../abstracts/' as *;
@use '../../vendors/bootstrap';

.school-space {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflexion : Est ce qu'on ajouterait pas "homepage" pour tous les composant de la home => "homepage-school-space" ?

export * from './SchoolSpace';
export { default as SchoolSpace } from './SchoolSpace';
export * from './SchoolSpaceContainer';
export * from './useUserSchools';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besoin de l'exporter ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants